Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(view): dont unwrap arrays in json mode #7506

Merged
merged 1 commit into from
May 11, 2024
Merged

fix(view): dont unwrap arrays in json mode #7506

merged 1 commit into from
May 11, 2024
+72 −12

Conversation

lukekarrys
Copy link
Contributor

@lukekarrys lukekarrys commented May 10, 2024 •
edited
Loading

The view command alters the data by default to unwrap single item arrays
when in human readable mode (the default). This change makes it so those
arrays are not altered when the --json flag is set.

Fixes #3611

# Before
❯ npm view tiny-tarball versions
1.0.0
❯ npm view tiny-tarball versions --json
"1.0.0"
# After
❯ npmlocal view tiny-tarball versions
1.0.0
❯ npmlocal view tiny-tarball versions --json
[
  "1.0.0"
]

@lukekarrys lukekarrys requested a review from a team as a code owner May 10, 2024 19:46
@lukekarrys lukekarrys changed the title fix(view): dont unwrap arrays in json modified fix(view): dont unwrap arrays in json mode May 10, 2024
@lukekarrys lukekarrys force-pushed the lk/view-versions-json branch 2 times, most recently from 986ac2a to 0be70c4 Compare May 10, 2024 19:50
@lukekarrys
fix(view): dont unwrap arrays in json mode
d3ec5b5 
The view command alters the data by default to unwrap single item arrays
when in human readable mode (the default). This change makes it so those
arrays are not altered when the --json flag is set.

Fixes #3611
@npm-cli-bot
Copy link
Collaborator

npm-cli-bot commented May 10, 2024 •
edited
Loading

no statistically significant performance changes detected

timing results
app-large clean lock-only cache-only modules-only no-lock no-cache no-modules no-clean show-version run-script cache-only
peer-deps		 no-clean
audit
npm@latest 35.938 ±1.02 10.448 ±0.06 11.486 ±0.04 1.552 ±0.00 1.519 ±0.00 1.266 ±0.01 8.016 ±0.03 1.290 ±0.02 0.138 ±0.00 0.165 ±0.01 13.049 ±0.13 4.171 ±2.96
#7506 39.301 ±0.37 10.557 ±0.05 11.589 ±0.07 1.538 ±0.04 1.524 ±0.01 1.260 ±0.00 8.147 ±0.07 1.277 ±0.01 0.137 ±0.00 0.163 ±0.00 14.469 ±0.14 2.112 ±0.02
app-medium clean lock-only cache-only modules-only no-lock no-cache no-modules no-clean show-version run-script cache-only
peer-deps		 no-clean
audit
npm@latest 30.000 ±0.97 7.860 ±0.02 8.758 ±0.00 1.488 ±0.02 1.502 ±0.00 1.396 ±0.00 5.801 ±0.01 1.326 ±0.01 0.139 ±0.00 0.169 ±0.00 9.375 ±0.20 2.988 ±1.46
#7506 29.355 ±0.96 7.845 ±0.04 8.810 ±0.04 1.492 ±0.02 1.487 ±0.00 1.395 ±0.02 5.777 ±0.01 1.288 ±0.03 0.135 ±0.00 0.164 ±0.00 9.640 ±0.05 1.947 ±0.00

@wraithgar
Copy link
Member

This feels like one of those incorrect behaviors folks have undoubtedly come to rely on, and this would be a breaking change.

@lukekarrys
Copy link
Contributor Author

lukekarrys commented May 11, 2024 •
edited
Loading

I'm not so sure, because currently this behavior is impossible to predict unless you know the data being returned. In the case of versions it will only unwrap the array if the package has only one version. It will properly keep it as an array for packages with multiple versions. So I think it's unlikely people are relying on this as they would still need to handle when versions returns an array.

# before
❯ npm view abbrev versions --json
[
  "1.0.3",
  "1.0.4",
  "1.0.5",
  "1.0.6",
  "1.0.7",
  "1.0.9",
  "1.1.0",
  "1.1.1",
  "2.0.0"
]
# after
❯ npmlocal view abbrev versions --json
[
  "1.0.3",
  "1.0.4",
  "1.0.5",
  "1.0.6",
  "1.0.7",
  "1.0.9",
  "1.1.0",
  "1.1.1",
  "2.0.0"
]

@ljharb
Copy link
Contributor

ljharb commented May 11, 2024

If they did the smart thing and always wrapped the field in [].concat() then this wouldn’t break them, at least :-)

@lukekarrys lukekarrys merged commit e40454c into latest May 11, 2024
23 checks passed
@lukekarrys lukekarrys deleted the lk/view-versions-json branch May 11, 2024 07:38
@github-actions github-actions bot mentioned this pull request May 11, 2024
Merged
@npm-cli-bot npm-cli-bot mentioned this pull request May 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wraithgar wraithgar wraithgar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] npm view $package versions --json returns non-JSON string for packages with only one version
4 participants
@lukekarrys @npm-cli-bot @wraithgar @ljharb